Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Assume Object/Array prototypes are sealed #822

Closed
wants to merge 3 commits into from

Conversation

catch-point
Copy link

This patch replaces ecma-5.js with utilities functions in tree.js. This allows LESS to run in non ECMA-5 environments where Object, Array, and Array.prototype are sealed.

@neonstalwart
Copy link
Contributor

fwiw, this would fix #99 which for some reason got closed without a fix

@lukeapage
Copy link
Member

Hi,

Excuse my ignorance (I really haven't investigated), what environments have sealed object and array prototypes?

And which old javascript engines have bugs that mean replace || with :?

Also the pull appears (at a cursery glance) to include other changes (e.g. https://github.com/cloudhead/less.js/pull/822/files#L2L900) or am I wrong?

@catch-point
Copy link
Author

On Mac OS X 10.7.4 (Lion) with Java(TM) SE Runtime Environment (build 1.6.0_33-b03-424-11M3720) and Java HotSpot(TM) 64-Bit Server VM (build 20.8-b03-424, mixed mode), in the included Language ECMAScript 1.6 implemention "Mozilla Rhino" 1.6 release 2 the following is true.

typeof(undefined || null) == "undefined"

However, in most ECMAScript implementations that would be false. This causes some issues with less.js when a undefined value is passed through a "var || null" expression. So I replaced all || and && expressions that did not evaluate to a boolean.

Those changes you point out are in the artifact less-1.3.0.js, which was modified when I ran "make". It looks like the artifact was not update with the current parser.js. So this pull requests also bring the artifact update (not just with these changes, but also with the entire source code base). I am not sure what the policy is for artifacts, but I am happy to exclude them if desired.

@@ -897,14 +781,38 @@ less.Parser = function Parser(env) {
elements.push(new(tree.Element)(c, e, i));
c = $('>');
}
$('(') && (args = $(this.entities.arguments)) && $(')');
if ($('(')) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comes from 1857b7c#L0R763

@dmcass
Copy link
Contributor

dmcass commented Aug 6, 2012

Array/Object prototypes are not sealed in Rhino 1.6r2, so there's still a question of whether or not this PR is necessary for it's title purpose. I'd prefer not to get into the debate point that was brought up in #99 (extending natives), and I'm assuming @cloudhead didn't want to either since he closed it without comment.

Regarding the other changes (changing default operators to ternary operators), this should probably be done in a different PR, since once again it's not related to the ES5 changes. In addition, this was fixed in Rhino 1.6r3. Rhino 1.6r2 is nearing 7 years old, is it still widely used enough to warrant supporting it?

Dist files should definitely be excluded from any PR since merging a PR does not initiate a new version or build.

@cloudhead
Copy link
Member

sorry, but no.

@cloudhead cloudhead closed this Aug 7, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants